I tried making an example test (this code is far too big of course!)

https://gist.github.com/ChrisJefferson/3d0813c40b130d1da5e04dc92cc5b36c -- also a second file containing a function id which just returns it's argument, just to stop the compiler optimising away the local variable.

With emcc -O1 this code fails, then passes with the "Spill Pointers" pass. With -O2 however it fails with "spill pointers", and more worryingly it output looks corrupted (the global pointer which should be constant changes value at some point while printing).

@ChrisJefferson Thanks!

The challenge will be testing it here in binaryen though. I'm not sure atm how to do that. I also probably won't have time to look into it soon, sorry, but perhaps someone else can take this PR as a starting point.

Thanks. Is there any place where a test might live? (I could take a look at existing tests), or is there not really a test framework to run an optimiser on some code, and check the result?

@ChrisJefferson Sorry for the late response. We do have tests to run the optimizer and see the code that results (tests/lit/, tests/passes/ - and this PR has a test there) but only running the code would have noticed the wrong stack direction... and that requires a full program that doesn't really fit in the test suite here. So this is an awkward situation, and I don't have a good idea.

However, this has come up again in emscripten-core/emscripten#17131 (comment) and so I think we should just land this, and figure out more detailed testing later (perhaps on the emscripten side?)

Would be nice to had a more readable/smaller test in tests/lit.. but not going to block this change on that.

Shame about re-introducing those two header files just for this one pass :(

Nice, thanks, getStackPointerGlobal makes this a lot cleaner...

I think in this PR it's best to keep the test identical to before, so this is a clean reversion of the removal. We can improve the test later if we want.

Updated after feedback.

Oh, we can at least remove one header here, I did that now: it is easy to get the pointer type since we've added indexType to memories (we added that for wasm64, I believe).